Use absolute paths in #line directives generated #9
Conversation
|
TestMergeSketch (file sketch_source_merger_test.go) now fails. Since it has to pass on different PCs and OSs, you should add a placeholder to merged_sketch.txt and replace it at test time using the actual absolute path sketch1.ino |
|
@ffissore, is there any test already with similar placeholders to copy from / keep consistency with? |
|
None. However there's an Abs helper func that you can use
|
Multiple .ino files in a sketch are concatenated together, adding #line directives so error messages refer to the original filenames. However, these directives used plain filenames, without a path. Since these filenames end up in the debug info as-is, this complicates using a debugger on the resulting .elf file. Using full pathnames fixes this. This fixes arduino/Arduino#3746. Signed-off-by: Matthijs Kooijman <[email protected]>
matthijskooijman
force-pushed
the
absolute-line
branch
from
225c9e9 to
14d364c
Compare
|
I pushed one more commit to fix the testcase. I ended up using the text/template package to allow interpolating anything from the context, which is (more than) sufficient for this usecase (and easier than implementing some string replace manually). If you think it's ok, I'll squash the commits together to prevent breaking the testcases in the first commit. |
|
Oh, seems there's more testcases breaking, hold on. |
matthijskooijman
force-pushed
the
absolute-line
branch
from
14d364c to
124e979
Compare
|
Ok, pushed a few more fixes, all tests succeed again now. |
|
Uhm are you sure? All tests from prototypes_adder_test.go are failing now |
This uses the text/template package to process the sketch file, allowing it access to the full context. This is a bit overkill, but it is easy and might be useful for more complex testcases later.
matthijskooijman
force-pushed
the
absolute-line
branch
from
124e979 to
dae09b9
Compare
|
Seems I did just Fixed now. |
Use absolute paths in #line directives generated
|
👍 |
|
Sorry @matthijskooijman, after I merged it I noticed your second commit was not signed off. I can't reopen this PR. Please make another one and ensure each of your commits is signed off. Might be worth installing this local git hook https://github.com/arduino/arduino-builder/wiki/Be-sure-your-commit-is-Signed-off-by |
|
W00ps! I'll submit a new PR with signoff in a minute. Perhaps it's also an idea to have github automatically check PRs for signoff lines? Seems github itself cannot do this, but it should be easy to use a webhook for this (see this example), or perhaps this can be added to ArduinoBot? |
|
Yes, that's part of an infrastructure work I plan to do next week |
|
Btw, I just realized I didn't sign off the commit because I wanted to squash it with the first commit (I left it as separate commits for easier review first). Also, this commit does have some side effects (like breaking errror highlighting and making the error display less clear, see https://github.com/arduino/Arduino/issues/3745#issuecomment-142365575), so some additional changes are needed in the IDE too. Let's track those in that issue, I'll re-open it. I've also modified to hook in the wiki page to only give a warning about improper messages, without actually breaking the commit. This still allows committing things for testing without having to sign it off, or allows locally commiting other people's code that you can't sign off. Requiring signoff for all commits will actually reduce its value, since people will then just signoff everything, even if not appropriate. |
Multiple .ino files in a sketch are concatenated together, adding #line
directives so error messages refer to the original filenames. However,
these directives used plain filenames, without a path. Since these
filenames end up in the debug info as-is, this complicates using a
debugger on the resulting .elf file. Using full pathnames fixes this.
This fixes arduino/Arduino#3746.
Signed-off-by: Matthijs Kooijman [email protected]